-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Adds Bitbucket PR Support: remotes, autolinks, home, etc. #4070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f1680a0 to
8d1586f
Compare
c8a2244 to
2050f43
Compare
2050f43 to
daf4b83
Compare
CHANGELOG.md
Outdated
| - Adds a _Switch Model_ button to the AI confirmation prompts | ||
| - Adds Windsurf support — closes [#3969](https://github.com/gitkraken/vscode-gitlens/issues/3969) | ||
| - Adds "pro" indicators for applicable integrations — closes [#3972](https://github.com/gitkraken/vscode-gitlens/issues/3972) | ||
| - Adds integration with Bitbucket Cloud by showing enrichhed links to PRs and issues [#4045](https://github.com/gitkraken/vscode-gitlens/issues/4045) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do a header line for the parent issue once all the BitBucket support is in, with sub-lines like this one covering each aspect of the integration. For now, this can stay out of this PR.
| @@ -0,0 +1,289 @@ | |||
| import type { HttpsProxyAgent } from 'https-proxy-agent'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency can this file follow the naming pattern of similar ones for other providers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| return undefined; | ||
| } | ||
| let hasReviews = false; | ||
| let hasRejections = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A change request isn't quite the same as a rejection IMO. Maybe hasChangeRequests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| undefined, | ||
| undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest surfacing to the team that we do not know mergeableState or viewerCanUpdate on PRs from BitBucket, because this means that several Launchpad categories, including Ready to Merge, Has Conflicts, Failing CI, won't work with BitBucket PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. There is also problem with assignees that does not exist in Bitbucket.
| pr.participants | ||
| ?.filter(prt => prt.role === 'REVIEWER') | ||
| .map(prt => fromBitbucketParticipantToReviewer(prt, pr.closed_by, pr.state)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be all reviewers assigned to the PR who's review state is "review requested" i.e. they have been requested to review but have not yet submitted a review for that request.
Is that what this filter is capturing? Note importantly that anyone who's review status is "approved, changes requested, commented, dismissed" etc. shouldn't be in this list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@axosoft-ramint So, we should show Pending only, right?
| owner: string, | ||
| repo: string, | ||
| id: string, | ||
| options: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An options param shouldn't be required here. Any required bits should be outside of the options object.
| options: { | |
| options?: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed. I've moved the baseUrl to required params.
daf4b83 to
1ff3232
Compare
1ff3232 to
07826fa
Compare
a1727c0 to
6844237
Compare
6844237 to
a328685
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit weird that we require qualifiers like "pull request #" for autolinks instead of having it work like GitHub, but we can address that separately.
867f9d2 to
36cc563
Compare
a328685 to
aedf600
Compare
Description
solves #4045
Adds enriched Bitbucket PRs to various places:
Checklist
Fixes $XXX -orCloses #XXX -prefix to auto-close the issue that your PR addresses